Skip to content

Conversation

@Ericson2314
Copy link
Member

Motivation

Since it is currently unix-only, we can use .native() not .string() for perf, and we don't have to worry about platform-specific conversions.

Context


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

@github-actions github-actions bot added the with-tests Issues related to testing. PRs with tests have some priority label Dec 15, 2025
@Ericson2314 Ericson2314 force-pushed the std-file-system-path-in-builder branch from aed78a6 to 496e8e2 Compare December 15, 2025 19:26
@Ericson2314 Ericson2314 force-pushed the std-file-system-path-in-builder branch from 496e8e2 to 9ef60a7 Compare December 30, 2025 19:36
Since it is currently unix-only, we can use `.native()` not `.string()`
for perf, and we don't have to worry about platform-specific
conversions.
@Ericson2314 Ericson2314 force-pushed the std-file-system-path-in-builder branch from 9ef60a7 to 04c0e34 Compare December 30, 2025 19:40
@Ericson2314 Ericson2314 added this pull request to the merge queue Jan 4, 2026
Merged via the queue into NixOS:master with commit fef2e2e Jan 4, 2026
15 checks passed
@Ericson2314 Ericson2314 deleted the std-file-system-path-in-builder branch January 4, 2026 23:25
@xokdvium
Copy link
Contributor

xokdvium commented Jan 5, 2026

if (!cgroupFS)
throw Error("cannot determine the cgroups file system");
auto rootCgroupPath = canonPath(*cgroupFS + "/" + rootCgroup);
auto rootCgroupPath = canonPath((*cgroupFS / rootCgroup).native());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be something like:

auto rootCgroupPath = canonPath((*cgroupFS / std::filesystem::path(rootCgroup).relative_path()).native());

artemist added a commit to artemist/nix that referenced this pull request Jan 5, 2026
A commit in NixOS#14800 broke tests around creating cgroups due to incorrect
path handling logic.
(See https://hydra.nixos.org/build/318367985/nixlog/11)

Fix that logic and represent cgroups as CanonPath.

Co-authored-by: John Ericson <[email protected]>
Co-authored-by: Sergei Zimmerman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

with-tests Issues related to testing. PRs with tests have some priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants